feat(types): support ns timestamp #19827

xxhZs · 2024-12-17T08:37:42Z

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

our timestamp only support ms, But our internal implementation is capable of ns, it's just that we automatically convert it to ms when we read it in, and this pr can read ns to support ns.

Checklist

I have written necessary rustdoc comments.
I have added necessary unit tests and integration tests.
I have added test labels as necessary.
I have added fuzzing tests or opened an issue to track them.
My PR contains breaking changes.
My PR changes performance-critical code, so I will run (micro) benchmarks and present the results.
My PR contains critical fixes that are necessary to be merged into the latest release.

Documentation

My PR needs documentation updates.

Release note

src/common/src/types/datetime.rs

wcy-fdu · 2024-12-18T09:26:33Z

I tested whether I could read nanosecond in the parquet file on this branch, and it seemed to work.

wcy-fdu · 2024-12-19T04:40:52Z

sorry, mis clicked update branch

wcy-fdu · 2024-12-19T04:56:52Z

cc @xiangjinwu PTAL❤️

xiangjinwu

Could you also include the testing SQLs you have tried?
Is any caller of to_protobuf/from_protobuf assuming that Timestamp would be fixed 64-bit? This assumption is no longer true.

xiangjinwu · 2024-12-19T06:06:19Z

src/common/src/types/datetime.rs

-            )
-        }
+        let dt = s
+            .parse::<jiff::civil::DateTime>()


Why do we need to introduce yet another dependency in addition to chrono and speedate?

Because chrono can't be parsed, and speedate is only parsed at the us level

src/common/src/types/datetime.rs

src/storage/hummock_sdk/src/compaction_group/hummock_version_ext.rs

xxhZs · 2024-12-23T07:05:15Z

Could you also include the testing SQLs you have tried?

added in ci

src/common/src/types/datetime.rs

src/storage/hummock_sdk/src/compaction_group/hummock_version_ext.rs

e2e_test/batch/types/timestamp_ns.slt.part

xiangjinwu · 2024-12-26T06:16:58Z

Btw you can fix the e2e test with rowsort.

https://www.sqlite.org/sqllogictest/doc/trunk/about.wiki

The default is "nosort". In nosort mode, the results appear in exactly the order in which they were received from the database engine. The nosort mode should only be used on queries that have an ORDER BY clause or which only have a single row of result, since otherwise the order of results is undefined and might vary from one database engine to another. The "rowsort" mode gathers all output from the database engine then sorts it by rows on the client side.

#6776

If there's rowsort, slt file will contain sorted results.

xiangjinwu · 2024-12-26T09:21:24Z

FYI regarding your latest e2e failure, it is actually a bug in our existing to_char implementation #19944:

1 BC in PostgreSQL is year 0 in chrono. Although chrono also provides DateLike::year_ce method, its strftime module does not provide a specifier to use year_ce instead of year.

Given it is out of scope of this PR, let's just replace the case with one without BC.

fxi ci fix ci fix ci

xiangjinwu

Mostly looks good to me. Just more to test:

distinct / group by / order by / max
extract with epoch / second / milliseconds / microseconds
make_timestamp

xiangjinwu · 2024-12-27T07:13:43Z

src/common/src/types/datetime.rs

-        }
+        let dt = s
+            .parse::<jiff::civil::DateTime>()
+            .map_err(|_| ErrorKind::ParseTimestamp)?;


Error message of ParseTimestamp shall be updated from up to 6 digits to up to 9 digits.

wcy-fdu · 2024-12-27T07:36:31Z

Thanks for the efforts. I will doing some testing on sink side later.

xiangjinwu · 2024-12-27T08:18:04Z

Is any caller of to_protobuf/from_protobuf assuming that Timestamp would be fixed 64-bit? This assumption is no longer true.

Answering my own question: there is but it is only used as a hint - so okay to be inaccurate.

to_protobuf is used here. The size_of::<T> may result in an inaccurate estimate for buffer capacity.

risingwave/src/common/src/array/primitive_array.rs

Lines 232 to 236 in 05397f9

    
           let mut output_buffer = Vec::<u8>::with_capacity(self.len() * size_of::<T>()); 
        
           for v in self.iter() { 
        
               v.map(|node| node.to_protobuf(&mut output_buffer)); 
        
           }

from_protobuf is used here. Movement of Cursor makes no assumption on item sizes.

risingwave/src/common/src/array/proto_reader.rs

Lines 69 to 76 in 05397f9

    
           for not_null in bitmap.iter() { 
        
               if not_null { 
        
                   let v = T::from_protobuf(&mut cursor)?; 
        
                   builder.append(Some(v)); 
        
               } else { 
        
                   builder.append(None); 
        
               } 
        
           }

src/expr/impl/src/scalar/extract.rs

fix comm fix ci

fix ci fix ci fx i fix ci fix ci fix ci fxi ci fix ci fix ci fix ci fix ci

Co-authored-by: congyi wang <[email protected]>

Co-authored-by: Xinhao Xu <[email protected]> Co-authored-by: congyi wang <[email protected]>

Co-authored-by: congyi wang <[email protected]>

pjpringle · 2025-01-11T03:14:02Z

Nice useful feature for trading use cases

support

efcb97b

xxhZs requested a review from a team as a code owner December 17, 2024 08:37

xxhZs requested a review from xxchan December 17, 2024 08:37

github-actions bot added the type/feature label Dec 17, 2024

graphite-app bot requested a review from a team December 17, 2024 09:25

hzxa21 requested a review from wcy-fdu December 18, 2024 03:40

fuyufjh reviewed Dec 18, 2024

View reviewed changes

src/common/src/types/datetime.rs Show resolved Hide resolved

fix comm

4aad830

Merge branch 'main' into xxh/support-timestamp-ns-1

9bf25a9

wcy-fdu enabled auto-merge December 19, 2024 04:39

wcy-fdu disabled auto-merge December 19, 2024 04:40

wcy-fdu added the need-cherry-pick-release-2.1 label Dec 19, 2024

wcy-fdu requested a review from xiangjinwu December 19, 2024 04:56

xiangjinwu reviewed Dec 19, 2024

View reviewed changes

xxhZs added 2 commits December 23, 2024 14:25

add ci

a6acfc2

fix comm

026e08a

fix fmt

f1cb8ab

xxhZs requested a review from xiangjinwu December 23, 2024 07:35

xiangjinwu reviewed Dec 24, 2024

View reviewed changes

src/common/src/types/datetime.rs Outdated Show resolved Hide resolved

src/storage/hummock_sdk/src/compaction_group/hummock_version_ext.rs Outdated Show resolved Hide resolved

e2e_test/batch/types/timestamp_ns.slt.part Outdated Show resolved Hide resolved

add doc + fix ci

d09d463

xxhZs force-pushed the xxh/support-timestamp-ns-1 branch from 26d9664 to 6b8af32 Compare December 26, 2024 07:49

fix ci

2c87c76

fxi ci fix ci fix ci

xxhZs force-pushed the xxh/support-timestamp-ns-1 branch from 6b8af32 to 2c87c76 Compare December 26, 2024 09:54

fix ci

d975bf0

xiangjinwu approved these changes Dec 27, 2024

View reviewed changes

fix comm

1571e8f

xiangjinwu reviewed Dec 30, 2024

View reviewed changes

src/expr/impl/src/scalar/extract.rs Outdated Show resolved Hide resolved

xxhZs enabled auto-merge December 30, 2024 05:22

g This is a combination of 2 commits.

a3cb6da

fix comm fix ci

xxhZs force-pushed the xxh/support-timestamp-ns-1 branch from 2a866bb to a3cb6da Compare December 30, 2024 05:23

Merge branch 'main' into xxh/support-timestamp-ns-1

b5bc1ef

xxhZs force-pushed the xxh/support-timestamp-ns-1 branch 3 times, most recently from 72a5ad9 to 12fb1f0 Compare December 30, 2024 08:09

fix ci

758cda7

fix ci fix ci fx i fix ci fix ci fix ci fxi ci fix ci fix ci fix ci fix ci

xxhZs force-pushed the xxh/support-timestamp-ns-1 branch from 8a5496d to 758cda7 Compare December 30, 2024 08:25

xxhZs added 3 commits December 30, 2024 16:37

fix ci

5d5b305

fix ci

28e556f

fix ci

0a4a5fa

xxhZs added this pull request to the merge queue Dec 30, 2024

Merged via the queue into main with commit b8c9c23 Dec 30, 2024
28 of 29 checks passed

xxhZs deleted the xxh/support-timestamp-ns-1 branch December 30, 2024 10:35

github-actions bot pushed a commit that referenced this pull request Dec 30, 2024

feat(types): support ns timestamp (#19827)

5732720

Co-authored-by: congyi wang <[email protected]>

github-actions bot mentioned this pull request Dec 30, 2024

feat(types): support ns timestamp (#19827) #19966

Merged

github-merge-queue bot pushed a commit that referenced this pull request Dec 31, 2024

feat(types): support ns timestamp (#19827) (#19966)

34d0c23

Co-authored-by: Xinhao Xu <[email protected]> Co-authored-by: congyi wang <[email protected]>

lmatz added the need-cherry-pick-release-2.2 label Jan 2, 2025

github-actions bot pushed a commit that referenced this pull request Jan 2, 2025

feat(types): support ns timestamp (#19827)

f8a68b7

Co-authored-by: congyi wang <[email protected]>

github-actions bot mentioned this pull request Jan 2, 2025

feat(types): support ns timestamp (#19827) #20004

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(types): support ns timestamp #19827

feat(types): support ns timestamp #19827

xxhZs commented Dec 17, 2024 •

edited

Loading

wcy-fdu commented Dec 18, 2024

wcy-fdu commented Dec 19, 2024

wcy-fdu commented Dec 19, 2024

xiangjinwu left a comment

xiangjinwu Dec 19, 2024

xxhZs Dec 23, 2024

xxhZs commented Dec 23, 2024 •

edited

Loading

xiangjinwu commented Dec 26, 2024

xiangjinwu commented Dec 26, 2024 •

edited

Loading

xiangjinwu left a comment

xiangjinwu Dec 27, 2024

wcy-fdu commented Dec 27, 2024

xiangjinwu commented Dec 27, 2024

pjpringle commented Jan 11, 2025

feat(types): support ns timestamp #19827

feat(types): support ns timestamp #19827

Conversation

xxhZs commented Dec 17, 2024 • edited Loading

What's changed and what's your intention?

Checklist

Documentation

wcy-fdu commented Dec 18, 2024

wcy-fdu commented Dec 19, 2024

wcy-fdu commented Dec 19, 2024

xiangjinwu left a comment

Choose a reason for hiding this comment

xiangjinwu Dec 19, 2024

Choose a reason for hiding this comment

xxhZs Dec 23, 2024

Choose a reason for hiding this comment

xxhZs commented Dec 23, 2024 • edited Loading

xiangjinwu commented Dec 26, 2024

xiangjinwu commented Dec 26, 2024 • edited Loading

xiangjinwu left a comment

Choose a reason for hiding this comment

xiangjinwu Dec 27, 2024

Choose a reason for hiding this comment

wcy-fdu commented Dec 27, 2024

xiangjinwu commented Dec 27, 2024

pjpringle commented Jan 11, 2025

xxhZs commented Dec 17, 2024 •

edited

Loading

xxhZs commented Dec 23, 2024 •

edited

Loading

xiangjinwu commented Dec 26, 2024 •

edited

Loading